Resource idempotency and other quality of life#33
Conversation
…ComputerScienceResources.com into Resource-idempotency
…ComputerScienceResources.com into Resource-idempotency
There was a problem hiding this comment.
Pull Request Overview
This PR implements resource idempotency to prevent duplicate resource creation and includes several quality-of-life improvements. The main goal is to make the application more robust when handling resource submissions and improve user experience.
Key changes:
- Added duplicate resource detection that returns existing resources instead of creating duplicates
- Extracted resource creation logic into a dedicated service class for better maintainability
- Added UI loading states and improved form submission handling
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/Services/ComputerScienceResourceService.php | New service class containing extracted resource creation and show page logic |
| app/Http/Controllers/ComputerScienceResourceController.php | Refactored to use service class and handle duplicate resource exceptions |
| app/Exceptions/Resources/*.php | Custom exceptions for resource validation and duplicate detection |
| tests/Feature/ComputerScienceResourceTest.php | Added tests for idempotency, image cleanup, and model deletion behavior |
| tests/Feature/ResourceEditsTest.php | Updated test to use unique image filenames and verify edit image cleanup |
| resources/js/Pages/Resources/Create.vue | Added loading state management for form submission |
| resources/js/Pages/Resources/Form/TagsFields.vue | Added disabled state for submit button during loading |
| resources/js/Components/Form/TagSelector.vue | Improved dropdown positioning and removed backspace tag deletion |
| resources/js/Components/Resources/ResourceEdit/ResourceEditsFAQ.vue | Fixed grammar error and added clarification about auto-upvoting |
| app/Http/Controllers/ResourceEditsController.php | Changed from copy to move operation for image handling |
| app/Http/Controllers/UpvoteController.php | Added TODO comment for refactoring |
| app/Http/Controllers/CommentController.php | Reordered log parameters for consistency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } catch (Throwable $e) { | ||
| DB::rollBack(); | ||
| // Attempt to remove the uploaded image if it was stored | ||
| if (isset($path) && $path) { | ||
| try { | ||
| Storage::disk('public')->delete($path); | ||
| } catch (Throwable $removeEx) { | ||
| Log::warning('Failed to remove image after exception', [ | ||
| 'user_id' => Auth::id(), | ||
| 'image_path' => $path, | ||
| 'error' => $removeEx->getMessage(), | ||
| ]); | ||
| } | ||
| } |
There was a problem hiding this comment.
The variable $path may not be defined in the catch block if an exception occurs before line 44. This will cause an 'undefined variable' error when checking isset($path).
| $response->assertStatus(422); // Validation error | ||
|
|
||
| // The image should not exist in storage | ||
| Storage::disk('public')->assertMissing('resource/'.$formData['image_file']->hashName()); |
There was a problem hiding this comment.
The test is checking for the image at 'resource/'.$formData['image_file']->hashName() but the actual storage path uses store('resource', 'public') which may generate a different path format. The assertion should use the actual stored path or verify no files exist in the resource directory.
| Storage::disk('public')->assertMissing('resource/'.$formData['image_file']->hashName()); | |
| $this->assertEmpty(Storage::disk('public')->allFiles('resource'), "Failed asserting that no image files exist in the 'resource' directory after failed resource creation."); |
1 participant
No description provided.